Codecov Report

Merging #343 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #343   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines          43     62   +19     
  Branches        7     10    +3     
=====================================
+ Hits           43     62   +19

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e6031f...df1a41c. Read the comment docs.

Note: That even having an outdated dom-testing-library works fine. I didn't think it would, but it makes sense now so that's cool :)

This looks good to go from my perspective. I'll leave it open for a while for folks to review if they want.

Saweeet! Off it goes.

🎉 This PR is included in version 6.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

Do we have to await fireEvent now since it's wrapped? Or does it work synchronously too?

@kentcdodds

you'll have to wrap fireEvent with act() if you have anything async going on.

await act(async () => {
  fireEvent(...)
})

you could write a helper of course

async function fireEventAsync(){
  await act(async () => {
    fireEvent(...)
  })
}
// ...
await fireEventAsync()

@threepointone it was already wrapped in the previous version https://github.com/kentcdodds/react-testing-library/blob/master/src/index.js#L100

Wondering if this changes the required usage to need await

We don't need a fireEventAsync, people can use the findBy* queries or one of the wait* utilities which are now all wrapped.

fireEvent was already wrapped. No need to await an act that is not async.

Bottom line: We're solid.

Oh haha I forgot facepalm your lib is awesome

@kentcdodds Can you please elaborate on how is findBy* useful for firing events? I am confused :)

findBy* is not useful for firing events, but you could do it for finding an element that loads asynchronously after an event is fired:

fireEvent.click(loadGreeting)
const greetingNode = await findByText(/hello/i)

Oh wow, this is going to take some time to process. So far I've been using getBy* most of the time and did not even know there is some findBy*. 😮

So to be clear, if I am doing something else besides finding elements, eg. expect(container.textContext)... AND there is some async code then I have to wrap fireEvent into extra await act? If that's the case, it's going to be fairly confusing, to be honest. Especially when someone else looks at those tests and some will be wrapped and some not.

No. You should never have to wrap a fireEvent call in act. We do that for you. You shouldn't have to wrap any code in act 99% of the time, so long as you're using react-testing-library utilities, you should be good.

Also, findBy* is only a few weeks old, so it's understandable that you don't know about it. Check the docs :)

Well, so I made some convoluted example, but I am doing similar stuff fairly often. Sadly it's not passing and do wonder what's is the proper way out of it.

const TestComponent = () => {
  const [state, setState] = React.useState(1)
  const onClick = () => {
    setImmediate(() => {
      setState(i => i + 1)
    })
  }

  return (
    <>
      <button onClick={onClick}>GO</button>
      <div>{state}</div>
    </>
  )
}

test('nay', () => {
  const { getByText, container } = render(<TestComponent />)
  fireEvent.click(getByText('GO'))
  expect(container.textContent).toBe('GO2')
})

There is also an attempt to solve it with findBy* which seems to work most of the time, but for some reason is flaky as it has also failed a couple of times when re-running it 🤔

Sorry, I should have probably made a new issue, but I am kinda tired here and heading to bed. That might be also a reason why I missed something there 🤷‍♂️

That's pretty weird. findBy* queries should reject if no element is found. Could you dig a little deeper and open an issue with your findings?

Sorry, have no capacity for diving deep in there right now. I don't think I will ever need findBy* as it feels weird to me. So far I had no need to wait for some element to appear.

I've updated the CodeSandbox with two passing tests and one failing (findByTitle). Dig into it or leave it, up to you ;)

https://codesandbox.io/s/6jpr2ow8mk